Remove TODO_BETA code comments #4076
Open
+10
−28
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3744
Notes
Three
TODO_BETA
comments were related to public sharing of tables/explorations. I removed those comment blocks entirely because I'm not sure we'll ever re-implement that, and if we do it'll require more than just a few small changes in those code locations.I changed
TOOD_BETA
toTODO
for smaller changes that seem worth doing at some point. If they are changes which should block beta, then I made sure that they are tracked in GitHub issues. For lower-priority changes, I didn't bother to ensure we have GitHub issues to track them. For exampleget(schema.name)
inSchemaSelector.svelte
seems okay for now. I tested renaming a schema to see if I could trigger a reactivity bug in the schema selector, and everything seems to function as I'd expect.I'm handling the TODO_BETA comment in
pageTitleUtils.ts
via Fix small problem with schema name in page titles #4074@pavish I'd like you to fix the
TODO_BETA
issue insrc/systems/data-explorer/utils.ts
. I know I asked you about this on a call months ago. I remember you telling me how to fix it. And I remember it making sense at the time. I'm sorry I never actually made a PR for it. Now I've forgotten. And I did try to figure it out again, but this code is pretty tough to work with. I think it would be faster for you to make this change. Can you please push a commit to this PR to deal with it? I can't tell if there's a regression here, which is why I'd like to handle this before beta.Checklist
Update index.md
).develop
branch of the repositoryDeveloper Certificate of Origin
Developer Certificate of Origin